Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Upgrade/emberjs 1.13.2 #113

Merged
merged 12 commits into from
Jul 10, 2015

Conversation

peec
Copy link
Contributor

@peec peec commented Jun 25, 2015

This PR will upgrade the ember version to 1.13.2 and also include changes to make paper compatible with 1.13.2.

Specifically this is done:

  • Change to 1.13.2 of emberjs.

  • The use of nearestOfType (paper-sidenav-toggle component) does not seem to work in 1.13.2. Instead a global event bus is used here. The event bus can come handy for other components as well. The event bus principle is explained quite nicely here. As of such one can now trigger from any component:

        paperEventBus: Ember.inject.service('paper-eventbus'),
        someAction: function () {
            this.get("paperEventBus").publish('paper:toggle-sidenav');
            // or 
            this.get("paperEventBus").publish('paper:expand-sidenav'); 
        }

    Obviously this event-bus is global, so it should only be used for global components. One could also make these events non-global by adding attribute to the event name. i.e. .publish('paper:toggle-sidenav-' + this.get("menuhandle")'). Though I do not think this is necessary for the sidenav component.
    EDIT: using parentView now.

  • Updated {{#if template}} to respective {{#if hasBlock}}. ( With respective mixin to add support for pre 1.13.X releases).

  • Updated ember-prism to 0.0.5.

peec added 2 commits June 25, 2015 03:59
… for nearestOfType. Use global event bus principles for toggle menu operation. The eventbus fits quite nice for such operation. Rewrite test for navmenu to conform to the new way of handling toggle of menu. The event bus can also be triggered from anywhere by using the new events: paper:toggle-sidenav and paper:expand-sidenav. This is the final commit as of 1.13.2 migration it seems. After testing everything seems to be fine.
@miguelcobain
Copy link
Collaborator

@peec Please read emberjs/ember.js#11170

tl;dr

  • Right now, we should use parentView
  • In the future we may be doing stuff like this:
{{#paper-nav-container as |c|}}
  {{#c.paper-sidenav}} {{!-- sidenav here --}} {{/c.paper-sidenav}}
  {{#c.paper-content}} {{!-- content here --}} {{/c.paper-content}}
{{/paper-nav-container}}

Can you please refactor this PR to use parentView instead?

@miguelcobain
Copy link
Collaborator

Also, does using hasBlock break when using Ember 1.12?
Check #86.

@peec
Copy link
Contributor Author

peec commented Jul 10, 2015

@miguelcobain

I have now done the following:

  • hasBlock: Created backwards compatible mixin so it doesn't break pre - 1.13.X releases.
  • Removed service principle from commits and added support for parentView
  • Upgraded to 1.13.3.

@miguelcobain
Copy link
Collaborator

@peec I think nearestOfType works fine now.

Also, in the hasBlock computed property we could check for a "_super" hasBlock property. If that exists, use it, otherwise use template.

@peec
Copy link
Contributor Author

peec commented Jul 10, 2015

@miguelcobain

Good idea, I tested with nearestOfType and it works fine now.

Added:

  • nearestOfType support again
  • Check if hasBlock exist, if it does use it otherwise use this.get("template")

@miguelcobain
Copy link
Collaborator

Please remove app/services/paper-eventbus.js.

@miguelcobain miguelcobain merged commit 87ef88e into adopted-ember-addons:master Jul 10, 2015
@miguelcobain
Copy link
Collaborator

Thanks @peec!

I opted to keep ember 1.12 for now. That should affect people's projects, because ember-paper is now compatible with 1.13 as well.

When we move to ember-cli 1.13.1 we use its ember version.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants